Skip to content

Include headers in response#533

Closed
matiassalles99 wants to merge 2 commits intonylas:mainfrom
tern-travel:main
Closed

Include headers in response#533
matiassalles99 wants to merge 2 commits intonylas:mainfrom
tern-travel:main

Conversation

@matiassalles99
Copy link
Contributor

@matiassalles99 matiassalles99 commented Sep 17, 2025

A few months ago we reached out to Kiran about the X-Retry-After header not being included in errors, so he made these changes #517

We are now trying to make use of these headers, but it looks like they're still not included, you can reproduce it with a similar script:

error = nil
mutex = Mutex.new

threads = 50.times.map do
  Thread.new do
    res = Nylas::V3::GetThread.call(user:, thread_id:) # this is a service we have which just makes a get_thread call
    unless res.success?
      mutex.synchronize { error ||= res.error }
    end
  end
end

threads.each(&:join)

# then run
error.headers # -> will be nil

Description

  • The PR that Kiran opened assumed that the response param, which gets handed over from method to method, was the full response, but it's actually the response body
  • Instead of refactoring most of the HttpClient class, I just added an optional headers param

License

I confirm that this contribution is made under the terms of the MIT license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@playerzero-ai
Copy link

playerzero-ai bot commented Sep 17, 2025

Pull Request Summary

There appear to be no significant changes made in this pull request.

Files Changed

File Name Summary
(none provided) No files listed. Paste filenames or a diff/commit and I’ll summarize each change concisely.

View more in PlayerZero
updated: Sep 17 @ 03:15 AM UTC

@samuelpx samuelpx requested review from AaronDDM and kraju3 and removed request for kraju3 September 17, 2025 12:44
@matiassalles99
Copy link
Contributor Author

matiassalles99 commented Sep 24, 2025

@AaronDDM @samuelpx is there anything I can help with to get this one merged?

@AaronDDM
Copy link
Contributor

AaronDDM commented Sep 24, 2025

Sorry @matiassalles99 - this totally slipped, on me. Taking a look. I will likely get it merged and a new release only next week however.

@AaronDDM
Copy link
Contributor

AaronDDM commented Sep 24, 2025

There seem to be some failing tests, could you take a look?

Thank you

@samuelpx
Copy link
Contributor

samuelpx commented Sep 24, 2025

@AaronDDM no actual tests are failling, CI results are just errors writing the XML coverage file:

` Finished in 0.15317 seconds (files took 0.90316 seconds to load)
247 examples, 0 failures

/opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/rexml-3.4.4/lib/rexml/parsers/baseparser.rb:271:in `pull_event': Malformed XML: No root
element (REXML::ParseException)
Line: 3
Position: 214
Last 80 unconsumed characters:

  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/rexml-3.4.4/lib/rexml/parsers/baseparser.rb:249:in `pull'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/rexml-3.4.4/lib/rexml/parsers/treeparser.rb:21:in `parse'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/rexml-3.4.4/lib/rexml/document.rb:468:in `build'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/rexml-3.4.4/lib/rexml/document.rb:105:in `initialize'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/simplecov-cobertura-2.1.0/lib/simplecov-cobertura.rb:37:in `new'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/simplecov-cobertura-2.1.0/lib/simplecov-cobertura.rb:37:in `result_to_xml'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/simplecov-cobertura-2.1.0/lib/simplecov-cobertura.rb:20:in `format'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/simplecov-0.22.0/lib/simplecov/result.rb:51:in `format!'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/simplecov-0.22.0/lib/simplecov/configuration.rb:197:in `block in at_exit'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/simplecov-0.22.0/lib/simplecov.rb:189:in `run_exit_tasks!'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/simplecov-0.22.0/lib/simplecov.rb:179:in `at_exit_behavior'
  from /opt/hostedtoolcache/Ruby/3.2.9/x64/lib/ruby/gems/3.2.0/gems/simplecov-0.22.0/lib/simplecov/defaults.rb:30:in `block in <top (required)>'

Error: Process completed with exit code 1.
`

Feel free to -f push this, I'll do a PR pinning REXML to 3.3.9 (downgrade), this fixed the issue locally on all tested versions

@AaronDDM
Copy link
Contributor

AaronDDM commented Sep 25, 2025

Addressed in #534

@AaronDDM AaronDDM closed this Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AaronDDM AaronDDM AaronDDM approved these changes

@kraju3 kraju3 Awaiting requested review from kraju3

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matiassalles99 @AaronDDM @samuelpx